Implement max HTLC sync functionality - #578
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a reconciliation path that keeps each managed node’s per-channel advertised max_htlc_msat aligned to a configurable fraction of channel capacity, executed during ChannelMonitorJob passes. This extends NodeGuard’s channel monitoring loop with an additional LND policy update capability (implemented via an updated UpdateChannelPolicy request builder).
Changes:
- Introduces
MaxHtlcSyncResultandILightningService.SyncChannelMaxHtlc(...), including auditing and “no-op” detection when already at target. - Extends
ILightningClientService.SetChannelFeePolicy(...)to optionally setmax_htlc_msat, and wires sync execution intoChannelMonitorJob. - Adds
MAX_HTLC_CAPACITY_RATIOconstant + env override and comprehensive unit tests around max-HTLC target resolution and job behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Services/LightningService.cs | Implements SyncChannelMaxHtlc, adds result enum, and integrates policy read/write + auditing. |
| src/Services/LightningClientService.cs | Extends SetChannelFeePolicy to optionally include MaxHtlcMsat in the LND policy update request. |
| src/Jobs/ChannelMonitorJob.cs | Calls max-HTLC reconciliation per channel, containing failures per-channel to avoid aborting the job. |
| src/Helpers/Constants.cs | Adds MAX_HTLC_CAPACITY_RATIO default and env-based configuration parsing. |
| test/NodeGuard.Tests/Services/LightningServiceTests.cs | Adds extensive test coverage for max-HTLC sync outcomes, bounds handling, and auditing behavior. |
| test/NodeGuard.Tests/Services/LightningClientServiceTests.cs | Updates tests for the new SetChannelFeePolicy signature (named client: arg). |
| test/NodeGuard.Tests/Jobs/ChannelMonitorJobTests.cs | Adds job-level tests verifying sync is called for each channel and failures are contained. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Jossec101
approved these changes
Aug 26, 2026
Contributor
|
LGTM, 0.99 its big imo but let's see |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sets each managed node's advertised max_htlc_msat to MAX_HTLC_CAPACITY_RATIO (default 0.99) of channel capacity, reconciled per channel on every ChannelMonitorJob pass.
Covers all channel types (NodeGuard-created, detected, and pre-existing) since the job reconciles whatever ListChannels reports; writes echo the live base fee / fee rate / timelock and omit inbound fees, so the dynamic fee engine's values are left untouched.